Skip to content

SSHFileSystem._info should strip the protocol from the path - #43

Merged
efiop merged 1 commit into
fsspec:mainfrom
ryaminal:patch-1
Apr 22, 2024
Merged

SSHFileSystem._info should strip the protocol from the path#43
efiop merged 1 commit into
fsspec:mainfrom
ryaminal:patch-1

Conversation

@ryaminal

Copy link
Copy Markdown
Contributor

This is based on some pain points i was having while using GenericFileSystem.
Looks like other FS implementations also strip first. Seems like a "safe" thing to do.

pytest passes... for what it's worth.

This is based on some pain points i was having and looking at other FS impls.
@ryaminal ryaminal changed the title [WIP] SSHFileSystem._info should strip the protocol from the path SSHFileSystem._info should strip the protocol from the path Apr 17, 2024
@shcheklein

Copy link
Copy Markdown
Collaborator

@ryaminal

This is based on some pain points i was having while using GenericFileSystem.

could you please give more details?

Looks like other FS implementations also strip first.

could you please share some links? thanks!

@ryaminal

Copy link
Copy Markdown
Contributor Author

@shcheklein, here are some answers to your questions.

Pain points

The pain points with sshfs and GenericFileSystem is sort of explained here.
To summarize: because the path passed to _info contained the protocol the isdir check in GenericFileSystem would fail. Once I added the _strip_protocol from this PR that problem went away.

Examples from other FS impls that strip in _info

@shcheklein

Copy link
Copy Markdown
Collaborator

Thanks @ryaminal for the context.

@martindurant @efiop please merge it and release when you have time.

@efiop
efiop merged commit 807e00b into fsspec:main Apr 22, 2024
@ryaminal
ryaminal deleted the patch-1 branch April 22, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants